Kotlin: Avoid infinite recursion when extracting recursive interfaces #20726
Conversation
igfoo
force-pushed
the
igfoo/ClassInstanceStack
branch
2 times, most recently
from
0ca6ebf to
95ef5c7
Compare
igfoo
force-pushed
the
igfoo/ClassInstanceStack
branch
from
95ef5c7 to
06218d8
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR addresses an infinite recursion issue that occurs when extracting recursive interfaces in Kotlin code. The fix adds a ClassInstanceStack to track classes being extracted and detects potential cycles, falling back to raw types when recursion is detected.
Key Changes:
- Introduced a
ClassInstanceStackutility class to track class extraction recursion - Modified the extraction logic to detect and avoid infinite recursion by using raw types
- Added test cases for both nested generic types and recursive interface hierarchies
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| java/kotlin-extractor/src/main/kotlin/utils/ClassInstanceStack.kt | New utility class that tracks class extraction state and detects cyclic type arguments |
| java/kotlin-extractor/src/main/kotlin/KotlinUsesExtractor.kt | Added avoidInfiniteRecursion method that uses the stack to detect cycles and fall back to raw types |
| java/kotlin-extractor/src/main/kotlin/KotlinFileExtractor.kt | Wraps supertype extraction in try-finally to manage the class instance stack |
| java/kotlin-extractor/src/main/kotlin/KotlinExtractorExtension.kt | Instantiates and threads the ClassInstanceStack through the extraction pipeline |
| java/kotlin-extractor/src/main/kotlin/ExternalDeclExtractor.kt | Accepts and passes through the ClassInstanceStack parameter |
| java/ql/test-kotlin2/library-tests/nested_types/test.kt | Test case for nested generic types |
| java/ql/test-kotlin2/library-tests/nested_types/types.ql | Query to verify nested type extraction |
| java/ql/test-kotlin2/library-tests/nested_types/types.expected | Expected results for nested types test |
| java/ql/integration-tests/kotlin/all-platforms/recursive_interfaces/*.java | Test cases with recursive interface hierarchies |
| java/ql/integration-tests/kotlin/all-platforms/recursive_interfaces/test.kt | Kotlin test file using the recursive interfaces |
| java/ql/integration-tests/kotlin/all-platforms/recursive_interfaces/types.ql | Query to verify recursive interface extraction |
| java/ql/integration-tests/kotlin/all-platforms/recursive_interfaces/types.expected | Expected results for recursive interfaces test |
| java/ql/integration-tests/kotlin/all-platforms/recursive_interfaces/test.py | Test configuration for the integration test |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| from Type t | ||
| where | ||
| t.getName().matches("%MyType%") and |
There was a problem hiding this comment.
Line 5's condition checking for %MyType% is redundant since line 6 already filters for specific type patterns that would include MyType. This creates a logical AND that makes line 5 unnecessary - any type matching the patterns in line 6 will contain MyType. Consider removing line 5 to simplify the query logic.
| t.getName().matches("%MyType%") and |
| private val stack: Stack<IrClass> = Stack() | ||
|
|
||
| fun push(c: IrClass) = stack.push(c) | ||
| fun pop() = stack.pop() |
There was a problem hiding this comment.
[nitpick] The pop() method returns the popped value but none of the call sites use the return value. Consider changing the return type to Unit to make the API clearer about its intent, or add a comment explaining why the return value is exposed if future use is anticipated.
| fun pop() = stack.pop() | |
| fun pop() { stack.pop() } |
| } else { | ||
| return pair | ||
| } |
There was a problem hiding this comment.
[nitpick] The else clause is unnecessary here since the if block returns. Remove the else keyword and unindent line 552 to reduce nesting and improve readability.
| } else { | |
| return pair | |
| } | |
| } | |
| return pair |
2 participants
It's not perfect, but I think a proper fix will need to wait until we think about this properly in the planned rewrite.